-
Notifications
You must be signed in to change notification settings - Fork 1
Fix: Check none value before length of vendor #895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds an early guard in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Failure. Coverage is below 90%.Diff Coverage
|
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/test_sageintacct/test_utils.py`:
- Around line 1401-1415: The test function
test_get_or_create_vendor_with_none_sanitized_name currently declares an unused
db fixture which triggers Ruff ARG001; fix by either removing the db parameter
and adding `@pytest.mark.django_db` above the function (and import pytest if
missing) so the test still has DB access, or keep the db parameter and silence
Ruff by appending a targeted noqa to the function definition (e.g., def
test_get_or_create_vendor_with_none_sanitized_name(db): # noqa: ARG001); choose
one approach and apply it consistently.
tests/test_sageintacct/test_utils.py
Outdated
| def test_get_or_create_vendor_with_none_sanitized_name(db): | ||
| """ | ||
| Test get or create vendor returns None when sanitize_vendor_name returns None | ||
| """ | ||
| workspace_id = 1 | ||
| intacct_credentials = SageIntacctCredential.objects.get(workspace_id=workspace_id) | ||
| sage_intacct_connection = SageIntacctConnector(credentials_object=intacct_credentials, workspace_id=workspace_id) | ||
|
|
||
| # Test with None vendor name - should return None | ||
| vendor = sage_intacct_connection.get_or_create_vendor(None, create=True) | ||
| assert vendor is None | ||
|
|
||
| # Test with vendor name that becomes empty after sanitization (only special chars) | ||
| vendor = sage_intacct_connection.get_or_create_vendor('!@#$%^&*()', create=True) | ||
| assert vendor is None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silence Ruff ARG001 for unused db fixture.
The new test doesn’t use db, so Ruff will keep flagging Line 1401. Either remove the fixture and mark the test as django_db, or add a targeted noqa.
💡 Suggested fix (mark + remove unused arg)
+@pytest.mark.django_db
-def test_get_or_create_vendor_with_none_sanitized_name(db):
+def test_get_or_create_vendor_with_none_sanitized_name():🧰 Tools
🪛 Ruff (0.14.13)
1401-1401: Unused function argument: db
(ARG001)
🤖 Prompt for AI Agents
In `@tests/test_sageintacct/test_utils.py` around lines 1401 - 1415, The test
function test_get_or_create_vendor_with_none_sanitized_name currently declares
an unused db fixture which triggers Ruff ARG001; fix by either removing the db
parameter and adding `@pytest.mark.django_db` above the function (and import
pytest if missing) so the test still has DB access, or keep the db parameter and
silence Ruff by appending a targeted noqa to the function definition (e.g., def
test_get_or_create_vendor_with_none_sanitized_name(db): # noqa: ARG001); choose
one approach and apply it consistently.
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff Coverage
|
* Fix: Check none value before length of vendor * added test * fixed test --------- Co-authored-by: Ashutosh619-sudo <[email protected]>
* Fix: Check none value before length of vendor * added test * fixed test --------- Co-authored-by: Ashutosh619-sudo <[email protected]>
Description
Please add PR description here, add screenshots if needed
Clickup
Please add link here
https://app.clickup.com/
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.